-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
105852 add specs for upstream failures in appointment booking flow #21377
base: master
Are you sure you want to change the base?
Conversation
f436fde
to
43f114d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes
spec/support/vcr_cassettes/vaos/eps/get_appointments/500_error.yml
Outdated
Show resolved
Hide resolved
spec/support/vcr_cassettes/vaos/eps/get_appointments/500_error.yml
Outdated
Show resolved
Hide resolved
@carlosfelix2 can you get yourself added to the GitHub Teams for your... team? Does that make sense? I see you're only only on two VA org teams, neither of which are associated with actual teams. Are you on https://github.com/orgs/department-of-veterans-affairs/teams/vfs-vaos? Message me on Slack if you have questions! |
Summary
This pull request reverts the use of a custom error class in the
Eps::AppointmentService
, which turned out to be interfering with the already present and abstracted error handling in the parent base service. I discovered the bug as I was adding unit tests for upstream failures which in turn verify that the appropriate response is propagated as interpreted from the existing error handling logic.Related issue(s)
Testing done
What areas of the site does it impact?
Error handling changes:
modules/vaos/app/services/eps/appointment_service.rb
: Removed generic rescue block to avoid swallowing exceptions and to let them propagate properly.modules/vaos/app/sidekiq/eps/eps_appointment_worker.rb
: Changed the rescue block to handleCommon::Exceptions::BackendServiceException
instead ofEps::AppointmentService::ServiceError
and updated the error message sent to users.Test additions:
modules/vaos/spec/requests/vaos/v2/eps_appointments_spec.rb
: Added new contexts to test 404 and 500 error responses when fetching EPS appointments.modules/vaos/spec/services/eps/appointment_service_spec.rb
: Updated test to expectCommon::Exceptions::BackendServiceException
instead ofEps::ServiceError
.modules/vaos/spec/sidekiq/eps_appointment_worker_spec.rb
: Added new contexts to test error handling for 404 and 500 error responses and updated the expected failure messages. [1] [2]VCR cassette updates:
spec/support/vcr_cassettes/vaos/eps/get_appointment/404.yml
: Added new VCR cassette to record 404 error response for fetching EPS appointments.spec/support/vcr_cassettes/vaos/eps/get_appointment/500.yml
: Added new VCR cassette to record 500 error response for fetching EPS appointments.Acceptance criteria
Requested Feedback